Updates to pyflyte init templating #1665
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: zeryx <1892175+zeryx@users.noreply.github.com>
Signed-off-by: zeryx <1892175+zeryx@users.noreply.github.com>
zeryx
requested review from
wild-endeavor,
kumare3,
eapolinario,
pingsutw and
cosmicBboy
as code owners
|
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov Report
@@ Coverage Diff @@
## master #1665 +/- ##
==========================================
- Coverage 71.03% 70.89% -0.15%
==========================================
Files 336 336
Lines 30765 30746 -19
Branches 5573 2503 -3070
==========================================
- Hits 21853 21796 -57
- Misses 8367 8384 +17
- Partials 545 566 +21
|
Signed-off-by: zeryx <1892175+zeryx@users.noreply.github.com>
zeryx
changed the title
eapolinario
reviewed
May 30, 2023
Comment on lines
+82
to
+85
| repo = Repo.clone_from(git_url, "temp_repo") | ||
|
|
||
| # Checkout to the branch | ||
| repo.git.checkout(branch) |
There was a problem hiding this comment.
you can pass the branch name directly in the call to Repo.clone_from.
Comment on lines
+95
to
+99
| # Copy the files from the source directory to the destination directory | ||
| shutil.copytree(src_path, dest_dir) | ||
|
|
||
| # Remove the temporary cloned repo | ||
| shutil.rmtree("temp_repo") |
There was a problem hiding this comment.
how about we use a TemporaryDirectory to hold the directory? This way we don't need to do this book-keeping and it also allows for this command to be run multiple times.
| @@ -68,7 +68,6 @@ | |||
| "docstring-parser>=0.9.0", | |||
| "diskcache>=5.2.1", | |||
| "cloudpickle>=2.0.0", | |||
| "cookiecutter>=1.7.3", | |||
|
cc @eapolinario / @neverett do you think you want to check this one in? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
This PR removes the cookiecutter templating system for
pyflyte init, and instead uses gitpython, without templating.This is part of a wider effort to replace our Dockerfile / docker_build.sh systems with the much simpler
ImageSpec.This PR depends on flyteorg/flytekit-python-template#40
This is a breaking change
All templates are now imagespec based, and cookiecutter is no longer used to generate custom readme titles or directory names.
Type
Are all requirements met?
Complete description
As discussed as part of the larger Flyte example improvement with newly introduced imagespec.